Add Authz revocation upon Cert revocation, by feature flag.#8799
Add Authz revocation upon Cert revocation, by feature flag.#8799ezekiel wants to merge 9 commits into
Conversation
ezekiel
changed the title
|
@ezekiel, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values. |
|
@ezekiel, this PR adds one or more new feature flags: RevokeAuthzsUponRevokeCert. As such, this PR must be accompanied by a review of the Let's Encrypt CP/CPS to ensure that our behavior both before and after this flag is flipped is compliant with that document. Please conduct such a review, then add your findings to the PR description in a paragraph beginning with "CPS Compliance Review:". |
|
CPS Compliance Review: Our CP/CPS does not directly discuss authorization revocation - there ARE important points about authorization re-use time frames, including in the Baseline Requirements 4.2.1. This flag does not modify authorization re-use time frames. After enabling this flag, authorizations may be revoked in a particular circumstance, which fully prevents their re-use regardless of their age. |
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
LGTM with a few small nits!
| return nil | ||
| } | ||
|
|
||
| func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, cert *x509.Certificate, smeta *sapb.SerialMetadata) { |
There was a problem hiding this comment.
The only thing this function uses smeta for is the regID, so just pass the bare regID instead.
| return nil | ||
| } | ||
|
|
||
| func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, cert *x509.Certificate, smeta *sapb.SerialMetadata) { |
There was a problem hiding this comment.
Add a doc comment describing this function and the fact that its calling contract requires that it be called as a background goroutine (otherwise the context timeout change will cause problems).
| "expirenow": ssa.clk.Now(), | ||
| "valid": statusUint(core.StatusValid), | ||
| "pending": statusUint(core.StatusPending), |
There was a problem hiding this comment.
nit: these three are out of order relative to the order they're used in the actual sequel statement.
| } | ||
| } | ||
|
|
||
| // waitForAuthzStatusChange uses an acme client to poll some(a slice of) |
There was a problem hiding this comment.
| // waitForAuthzStatusChange uses an acme client to poll some(a slice of) | |
| // waitForAuthzStatusChange uses an acme client to poll some (a slice of) |
same below
| t.Helper() | ||
|
|
||
| for try := range 4 { | ||
| time.Sleep(core.RetryBackoff(try, time.Second, 2*time.Second, 1.5)) |
There was a problem hiding this comment.
I'd say you can start much faster than time.Second; the vast majority of integration test operations are probably complete within double-digit milliseconds.
Because it can revoke multiple authorizations. Co-authored-by: Aaron Gable <aaron@letsencrypt.org>
2 participants
No description provided.